Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pam/gdm): Fix and test handling of gdm qrcode regeneration #402

Merged
merged 14 commits into from
Jul 4, 2024

Conversation

3v1n0
Copy link
Collaborator

@3v1n0 3v1n0 commented Jun 27, 2024

Ensure reselectAuthMode is properly handled by the gdm model, following what the others model do

UDENG-3126

@3v1n0 3v1n0 requested a review from a team as a code owner June 27, 2024 18:25
@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.

Project coverage is 84.40%. Comparing base (d3d60d3) to head (f1bb410).

Files Patch % Lines
pam/internal/pam_test/pam-client-dummy.go 28.57% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #402   +/-   ##
=======================================
  Coverage   84.39%   84.40%           
=======================================
  Files          77       77           
  Lines        6691     6713   +22     
  Branches       75       75           
=======================================
+ Hits         5647     5666   +19     
- Misses        732      734    +2     
- Partials      312      313    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@3v1n0 3v1n0 force-pushed the qrcode-regenerate-cleanups branch 7 times, most recently from b3ab93c to 5e0edc5 Compare June 27, 2024 23:04
@denisonbarbosa
Copy link
Member

I'll convert this to draft after what we discussed during the meeting regarding splitting the content into different PRs.

@denisonbarbosa denisonbarbosa marked this pull request as draft June 28, 2024 14:41
@denisonbarbosa denisonbarbosa removed the request for review from a team June 28, 2024 14:41
@3v1n0 3v1n0 force-pushed the qrcode-regenerate-cleanups branch 3 times, most recently from 58130e7 to f1bb410 Compare July 2, 2024 17:51
@3v1n0 3v1n0 marked this pull request as ready for review July 2, 2024 18:51
@3v1n0 3v1n0 requested a review from denisonbarbosa July 2, 2024 18:52
3v1n0 added 13 commits July 3, 2024 17:49
We already write a similar output in conversation, and that's enough
…ction in such case

If we're already about to fail because of a timeout don't cleanup the
transaction because we may end up stopping ongoing conversations which
will lead to a panic, without clear explaination
This is an expectation, not really something we're doing in the test so
change the value as expected
Since we are sharing the daemon we should not reuse the same user
multiple time, so generate them based on the test name to avoid even
manual clashes
In gdm when the authentication stage is changed we reset the currently
svaed states, so do it also in the integration test mock to respect what
the UI would do
Sometimes we may want to respond to a specific gdm event with multiple
poll responses, to allow this, without refactor the world, we can just
create a simple fake event and use its type as a counter for the events
we want to group.

When preceding events with this fake event, then the following N events
will be added to the poll responses
Cancellation needs to be possible everytime we're in challenge mode, but
this is handled already by the authentication backend so don't bother
adding the logic here.

Since in this way we'll prevent being able to cancel authentications
that are in "wait" state and so when we're not anymore waiting for user
authentication request, but only for the broker reply.
After this happens we're gonna receive a further start authentication
event, so we need to stop considering the model being in auth phase
When waiting we may want to be able to cancel it, so support it
@3v1n0 3v1n0 force-pushed the qrcode-regenerate-cleanups branch from f1bb410 to fe1e2d5 Compare July 3, 2024 15:54
@3v1n0 3v1n0 merged commit 8e73e09 into ubuntu:main Jul 4, 2024
5 checks passed
@3v1n0 3v1n0 mentioned this pull request Jul 9, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants